-
Notifications
You must be signed in to change notification settings - Fork 388
Improve telemetry for firewall setup #3526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
azurelinuxagent/ga/env.py
Outdated
| OK = "OK" # The firewall rules for the WireServer are setup correctly | ||
| NotSet = "NotSet" # The firewall rules have not been set | ||
| Invalid = "Invalid" # The state of the firewall rules is not as expected, e.g. because some rules are missing | ||
| Inconsistent = "Inconsistent" # The stare of the firewall is reported differently by different tools, e.g. "iptables -C" vs "iptables -L" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks; fixed
azurelinuxagent/ga/env.py
Outdated
| self._firewall_manager = None # initialized on demand in the _operation method | ||
| self._message_count = 0 | ||
| self._report_after = datetime.datetime.now(UTC) | ||
| self._firewall_state = FirewallState.OK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we initialize to Unknown state since it hasn't been ch.cked yet?
class FirewallState(object):
...
Unknown = "Unknown" # The state of the firewall has not been checked yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initialized it to OK to avoid turning on verbose mode in the first iteration:
self._firewall_manager.verbose = self._firewall_state != FirewallState.OK and self._should_report
I can add a comment to that respect in the initialization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel first iteration may be critical given the existing duplicate cases we have seen so far, where env thread sees different result first time than we expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the initial setup is verbose, the initial periodic check is not - if the condition persists, the 2nd iteration of the env thread should produce verbose output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maddieford - added comment
(cherry picked from commit 41be93f)
Added a periodic report of the status of the firewall rules.
Also, if the status is incorrect, we use a "verbose" mode where all the commands related to the firewall are logged, along with their output.